feat(telegram): add inline keyboard to /session_recent, accept plain_dir in topic adoption - #3904
feat(telegram): add inline keyboard to /session_recent, accept plain_dir in topic adoption#3904gimso2x wants to merge 2 commits into
Conversation
aa33a4b to
08e1222
Compare
yazzang-homelab
left a comment
There was a problem hiding this comment.
Reviewed as independent architect review. CI is green and the head contains current dev, but there are three things to fix before this should land.
1. Blocking — /session_recent silently lost disable_notification.
The old replyHtml helper carried telegramDisableNotification(this.opts.sound, "finalized"). The new inline sendMessage loop does not:
await this.botApi.call("sendMessage", {
chat_id: this.opts.chatId,
...(threadId !== undefined ? { message_thread_id: threadId } : {}),
text: chunks[i]!,
parse_mode: TELEGRAM_PARSE_MODE,
...(i === chunks.length - 1 ? { reply_markup: { inline_keyboard } } : {}),
});So a user who configured silent finalized notifications now gets a push for every /session_recent. The sibling reply helper right above still honours it, which makes the inconsistency easy to miss in review. Restore the flag (or keep replyHtml and give it an optional reply_markup parameter — that also removes the duplicated chunk loop).
2. Blocking — CHANGELOG.
This is user-visible twice over (new inline keyboard on /session_recent, and topic adoption now accepts a non-existent directory). packages/coding-agent/CHANGELOG.md is untouched; the PR checklist item is unticked.
3. Please split — the plain_dir change is a policy change wearing a UI change's PR.
existing_path → plain_dir in topic adoption means a Telegram message now causes fs.mkdirSync(cwd, { recursive: true }) in lifecycle-control-runtime.ts. The test being flipped was an explicit assertion (pending topics reject dir and worktree creates instead of spawning duplicate topics), so this is deliberately reversing a prior decision — that deserves its own PR with its own reasoning, not a bullet under a keyboard feature. Concretely, please state in that PR: what bounds the created path (the chat is full-trust, but normalizeLifecyclePath accepts any absolute path, so /session_create path /etc/whatever now creates a directory), and what happens to the directory if session creation subsequently fails — is it left behind?
Non-blocking notes:
e.sessionId.slice(0, 12)in the button payload is fine —validateLifecycleTargetroutes resume throughsessionIdOrPrefix— but the test fixtures use 5-char ids (s-019), so truncation is never actually exercised. Worth one case with a full-length session id.- Telegram caps
inline_keyboardrows in practice;recent.entriesis unbounded here. Cap the button list (the text list can stay long since it already chunks). - Only the last chunk carries
reply_markup, which is correct, but the chunk loop no longer awaits ordering failures — a rejected middle chunk is swallowed by.catch(() => undefined)and the keyboard still posts. Pre-existing pattern, just noting it now applies to a multi-message body.
gajae.pr-review-verdict.v1 merge-blocked sha256:c19e8ecf2e895399636e22399923c4f12841773f reviewer:architect evidence:https://github.com/Yeachan-Heo/gajae-code/pull/3904/files
…dir in topic adoption - /session_recent: inline keyboard with switch_inline_query_current_chat buttons inserts /session_resume <id> into chat input, no callback polling needed - topic adoption direct path input: plain_dir instead of existing_path missing directories are created by the lifecycle orchestrator - combined double .map() over recent.entries into single for-loop pass Rejected: multibyte-safe title slice | cosmetic, Telegram enforces 1-256 char limit Confidence: high Scope-risk: narrow Reversibility: clean Tested: bun test (558 pass, 0 fail) Not-tested: runtime end-to-end Telegram API integration
- /session_recent rewrite left replyHtml unused; biome check failed - TelegramAdoptionTarget and persisted-intent guard now accept plain_dir so topic-adoption direct paths survive daemon restart
c19e8ec to
4236fb7
Compare
|
Closing during the emergency maintenance freeze. This PR is not in the retained critical or maintainer-owned set. Do not open a replacement PR unless a maintainer explicitly directs it. — |
What
/session_recent: inline keyboard buttons withswitch_inline_query_current_chat— tap a session button to insert/session_resume <id>into chat input, no callback polling neededexisting_path→plain_dirso missing directories are auto-created by the lifecycle orchestrator.map()overrecent.entriesinto single for-loop passWhy
~/docsgot "folder no longer available" when the directory didn't exist — now it is created automaticallyTesting
bun test notifications-telegram-daemon.test.ts notifications-lifecycle-command-routing.test.ts→ 514 pass, 0 failplain_dirtargetGJC verdict